[LXC] Address file system policy gaps by dhoehna · Pull Request #630 · microsoft/mxc

dhoehna · 2026-07-10T22:56:08Z

Linked work item: AB#62861419 — [LXC] Address file system policy gaps

Summary

Hardens LXC denied-path masking and adds a reusable most-specific-path-wins policy resolver in wxc_common. No schema or wire-format changes — see the Schema note below.

Denied-path masking (host reality): the LXC denied branch now classifies each denied path from the host path (denied paths are host paths) instead of the container rootfs path. The rootfs path (<lxc_path>/<name>/rootfs/<container_path>) does not exist until mounts are applied, so inspecting it always misclassified the mask as a directory. A regular host file is now masked with a read-only /dev/null bind (create=file); a directory, symlink, or missing host path with an empty read-only tmpfs (create=dir). Classification uses symlink_metadata so a symlinked deny is never followed to an unintended target.
Most-specific-path-wins resolver: extends the existing filesystem_resolve module in wxc_common in place (no module rename / compat shim). Paths are emitted shallow-to-deep so the deepest (most specific) intent wins at every path regardless of which policy list it came from (e.g. readwritePaths: [""/data/secrets""] survives deniedPaths: [""/data""]). Exact same-path conflicts are not re-collapsed in the resolver — that is already handled upstream by the config parser's normalize_filesystem_paths (string-level) and each runner's normalize_object_conflicts (object-identity); for any duplicate that still reaches the resolver, emission order (denied emitted last) preserves most-restrictive-wins.

Schema

No schema or wire-format changes. The earlier type: ""file"" | ""dir"" discriminator — and the MaskKind / denied_path_kinds model fields — have been dropped. Per the discussion on this PR, the file/dir (and leaf/tree) distinction will be handled as separate schema work, and the shared schema/wire surface is owned by #640 (user/sodas/bwrap-denied-path-type), which LXC will consume once it lands. This PR is scoped to the resolver + host-reality masking and touches no files under schemas/ or sdk/src/generated/.

Validation

cargo fmt --all -- --check — clean
cargo clippy -p wxc_common --all-targets -- -D warnings (Windows) and cargo clippy --target x86_64-unknown-linux-gnu -p lxc_common -p bwrap_common --all-targets -- -D warnings — clean
cargo test -p wxc_common — 397 passed
cargo check for lxc_common + bwrap_common on x86_64-unknown-linux-gnu — pass

Independent of the sibling LXC branches (no shared model fields).

Microsoft Reviewers: Open in CodeFlow

…B#62861419) - Replace is_file() masking heuristic with explicit `type` schema field + symlink_metadata() fallback (no symlink follow; fail-closed on missing+untyped). - Add reusable most-specific-path-wins resolver in wxc_common (deny>ro>rw); wire LXC mounts to emit in specificity order. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3b78bec0-e139-4cfd-9c10-092ef986d4f4

Copilot

Pull request overview

This PR hardens filesystem policy handling by extending the wire format for filesystem.deniedPaths and introducing a reusable “most-specific-path-wins” resolver to produce deterministic mount emission order for Linux backends.

Changes:

Extend deniedPaths to accept either a legacy string or an object { path, type: "file" | "dir" }, and plumb the parsed mask kind into ContainerPolicy.
Add wxc_common::path_specificity to resolve overlapping filesystem intents by specificity (deep overrides shallow; exact ties pick most restrictive).
Update LXC mount emission to use the new resolver and apply explicit denied-path mask kinds.

Reviewed changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/core/wxc_common/src/wire.rs	Changes wire model so `deniedPaths` supports string or typed object entries.
src/core/wxc_common/src/ts_emit.rs	Updates TS emitter to correctly emit `anyOf` unions and `allOf` wrappers as TypeScript type aliases.
src/core/wxc_common/src/path_specificity.rs	Introduces the new most-specific-path-wins resolver and tests.
src/core/wxc_common/src/models.rs	Adds `MaskKind` and `denied_path_kinds` to `ContainerPolicy` for backend masking decisions.
src/core/wxc_common/src/lib.rs	Exposes the new `path_specificity` module.
src/core/wxc_common/src/filesystem_resolve.rs	Keeps compatibility by re-exporting the resolver from the new module.
src/core/wxc_common/src/config_parser.rs	Converts wire denied-path entries into `denied_paths` + `denied_path_kinds` in the domain policy.
src/backends/lxc/common/src/filesystem_mounts.rs	Switches mount emission ordering to the resolver and adds explicit denied-path masking logic.
sdk/src/generated/wire.ts	Regenerates TS wire types for the new `DeniedPath` union/object forms.
schemas/dev/mxc-config.schema.0.8.0-dev.json	Regenerates the dev schema to reflect the new denied-path shapes.

dhoehna · 2026-07-13T21:08:58Z

+fn observed_mask_path_kind(full_path: &str) -> Result<Option<ObservedMaskPathKind>, String> {
+    match std::fs::symlink_metadata(full_path) {
+        Ok(metadata) => {
+            let file_type = metadata.file_type();
+            if file_type.is_dir() {
+                Ok(Some(ObservedMaskPathKind::Dir))
+            } else if file_type.is_symlink() {
+                Ok(Some(ObservedMaskPathKind::Symlink))
+            } else {
+                Ok(Some(ObservedMaskPathKind::File))
+            }
+        }
+        Err(err) if err.kind() == std::io::ErrorKind::NotFound => Ok(None),
+        Err(err) => Err(format!(
+            "Unable to inspect denied path '{}': {}. Set deniedPaths entry to an object with explicit type \"file\" or \"dir\".",
+            full_path, err
+        )),
+    }
+}


Fixed in b7bfe39. observed_mask_path_kind now takes &Path and inspects the host path (Path::new(host_path)) directly instead of the container rootfs path. Denied paths are host paths, so classifying them off the rootfs path (which doesn't exist before mounts are applied) forced a spurious NotFound and demanded an explicit type. Passing &Path also drops the to_string_lossy() allocation.

dhoehna · 2026-07-13T21:09:00Z

+                let rootfs_base = format!("{}/{}/rootfs", container.lxc_path(), container.name());
+                let full_path = Path::new(&rootfs_base).join(container_path);
+                let explicit = policy.denied_path_kinds.get(host_path).copied();
+                let kind = resolve_mask_kind(
+                    host_path,
+                    explicit,
+                    observed_mask_path_kind(&full_path.to_string_lossy())?,
+                )?;


Fixed in b7bfe39. Mask-kind observation now uses the host path instead of .../rootfs/<container_path>, so host paths that don't yet exist under rootfs (including the temp paths substituted into tests/configs/lxc_filesystem_object.json by tests/scripts/run_lxc_object_test.sh) no longer hard-error.

I also added lookup_mask_kind, which tries an exact match first and then falls back to a trailing-separator–normalized comparison, so an explicit type isn't dropped when the resolved mount path and the configured denied_path_kinds key differ only by a trailing slash. Added unit tests for host-path observation (dir/file/missing) and the normalized lookup.

…h keys Address PR review feedback on denied-path masking: - `observed_mask_path_kind` now takes `&Path` and inspects the *host* path instead of the container rootfs path (`.../rootfs/<container_path>`). The rootfs path does not exist before mounts are applied, so the previous code returned `NotFound` for real host denied paths and spuriously demanded an explicit `type`, breaking existing LXC object-policy validation. Also drops the `to_string_lossy()` allocation by passing `&Path` directly. - Add `lookup_mask_kind` with trailing-separator normalization so an explicit `type` is not dropped when the resolved mount path and the configured `denied_path_kinds` key differ only by a trailing slash. - Add unit tests for host-path observation (dir/file/missing) and normalized kind lookup. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

SohamDas2021 · 2026-07-13T23:35:06Z

        }
      ]
    },
+    "DeniedPath": {


This will conflict with the bwrap implementation- https://github.com/microsoft/mxc/pull/640/changes#diff-1cd186aad0db50fef812edec1d51621e632e26ed32f8… as I had already mentioned the dependency in the task. There is no need to make this change again. I will merge the bwrap PR first

You may want to base your changes off user/sodas/bwrap-denied-path-type and reuse the schema changes

Schema/wire layer is shared across all backends. This is exactly the shared surface wxc_common exists to own.
You have to educate LXC runner to just use this instead.

I just had a chat with Gudge- "BTW I've blocked #640. I want to do schema work like that separately. And if we're adding file/dir, we should probably think about adding leaf/tree at the same time. They are both on my file system policy backlog."

Let's hold off to making the file vs dir distinction for now in the schema. You can continue to work on the remaining fs gaps.

Done — the file/dir type distinction is dropped from the schema entirely in b0a5766. The generated dev schema (schemas/dev/mxc-config.schema.0.8.0-dev.json) and SDK wire types (sdk/src/generated/wire.ts) are reverted to the #608 base, and deniedPaths is back to bare strings. Agreed on holding off per Gudge — I'll leave the file-vs-dir (and leaf/tree) design to the separate schema work and consume the shared schema/wire surface from #640 / user/sodas/bwrap-denied-path-type once it merges. This PR is now scoped to the remaining fs gap: the most-specific-path resolver. Codegen gates (check-schema-codegen / check-sdk-types-codegen / check-schema-versions / validate-configs) all pass.

SohamDas2021 · 2026-07-13T23:59:11Z

Since this might be touching common code, make sure all lxc and bwrap tests/configs pass

SohamDas2021 · 2026-07-13T23:44:02Z

+#[derive(Debug, Clone, Serialize, Deserialize)]
+#[cfg_attr(feature = "schema-gen", derive(schemars::JsonSchema))]
+#[serde(untagged)]
+pub enum DeniedPath {


Again all these changes are already made in my bwrap PR- #640. You would need to use that.

Removed. All wire.rs changes for the denied-path type union (DeniedPath, MaskKind, denied_path_kinds) are reverted to base in b0a5766, so this no longer duplicates the bwrap #640 wire surface. LXC will consume the shared wxc_common wire types from #640 once it lands rather than redefining them here.

SohamDas2021 · 2026-07-13T23:51:11Z

+    explicit: Option<MaskKind>,
+    observed: Option<ObservedMaskPathKind>,
+) -> Result<MaskKind, String> {
+    if let Some(kind) = explicit {


The Bubblewrap side ( denied_masks_as_file ) does the opposite: when the host path exists, host reality wins and a contradicting declared type is downgraded to a warning- specifically to avoid emitting an invalid mount (e.g. binding /dev/null over a real directory). Here, a config declaring type:"file" on a path that is actually a directory will emit /dev/null … create=file over it. For the same schema field to behave oppositely on LXC vs Bubblewrap is a cross-backend correctness inconsistency.

Can we replicate bwrap behavior here?

Resolved by removal — the divergent LXC mask-kind logic is gone in b0a5766. The denied-path branch is reverted to its original host-reality masking (/dev/null for a file, tmpfs for a directory) with no declared-type override, so there's no longer a cross-backend inconsistency with bwrap's denied_masks_as_file. When the shared file/dir surface arrives via #640, LXC will adopt bwrap's semantics (host reality wins; a contradicting declared type is downgraded to a warning) instead of reintroducing the divergent behavior here.

SohamDas2021 · 2026-07-13T23:56:02Z

+            .then_with(|| b.intent.cmp(&a.intent))
+            .then_with(|| a.sequence.cmp(&b.sequence))
+    });
+    candidates.dedup_by(|a, b| {


In Vec::dedup_by, when the closure returns true, a (the later element) is removed and b is kept; so, mutating a has no effect on the retained element. It's dead code. The "most-restrictive-wins" result only survives because the preceding sort_by already places the most-restrictive entry first (b). Either drop the mutation (rely on the sort) with a comment or mutate b . Please also add a direct test asserting the kept path string on an exact-key conflict across lists that differ only by trailing slash.

Fixed in b0a5766. Dropped the dead mutation of a and now rely on the preceding most-restrictive-first sort_by, with a comment explaining that Vec::dedup_by keeps the earlier element (b) and removes the later a. Added a direct test — exact_key_conflict_keeps_most_restrictive_path_string — asserting the kept path string when two lists collide on the same normalized key but differ only by a trailing slash. Per the module-rename thread this now lives in filesystem_resolve.rs. cargo test -p wxc_common passes (398 tests).

SohamDas2021 · 2026-07-13T23:56:36Z

@@ -0,0 +1,266 @@
+// Copyright (c) Microsoft Corporation.


Renaming a shared module that was introduced in #608, and leaving a permanent compat shim, is avoidable churn and increases conflict surface with the Bubblewrap branches. Prefer extending filesystem_resolve in place.

Reverted the rename. path_specificity.rs and the compat shim are deleted; the enhanced resolver (exact-key dedup + the new kept-path test) now extends filesystem_resolve in place, so #608's module keeps its name and there's no added conflict surface with the Bubblewrap branches. Consumers (LXC filesystem_mounts.rs and bwrap bwrap_command.rs) import from wxc_common::filesystem_resolve unchanged.

Address review feedback on microsoft#630: - Remove the denied-path file/dir `type` discriminator (wire DeniedPath union, MaskKind, denied_path_kinds, regenerated schema + TS wire types, ts_emit anyOf/allOf handling, and the LXC mask-kind logic). Per reviewer guidance the file-vs-dir schema distinction is deferred (to be designed alongside leaf/tree), and the wire change duplicates bwrap microsoft#640. - Keep the most-specific-path-wins resolver but extend `filesystem_resolve` in place instead of renaming it to `path_specificity` with a compat shim, avoiding churn / conflict surface with the bwrap branches. Delete path_specificity.rs and the shim. - Fix the `dedup_by` dead code: the retained element is the earlier `b`, so mutating `a` was a no-op. Rely on the most-restrictive-first sort (with a comment) and add a test asserting the kept path string on an exact-key conflict across lists that differ only by a trailing slash. - Revert LXC denied-path masking to its original behavior while still emitting mounts in resolver (shallow->deep) order. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

SohamDas2021 · 2026-07-14T20:06:00Z

The PR description is stale and still says that you update the schema.

SohamDas2021 · 2026-07-14T20:10:11Z

+/// of the deepest plan entry that is an ancestor of (or equal to) `path`, with
+/// most-restrictive-wins breaking an exact-depth tie. `None` if no entry covers
+/// the path.
+pub fn effective_intent(plan: &[ResolvedMount], path: &str) -> Option<FsIntent> {


unused? Only in tests? Remove

Done in bae58f5. effective_intent was only a test oracle (no production/public caller), so it's moved into filesystem_resolve's #[cfg(test)] mod tests, and PathKey::is_prefix_of (its only caller) is now #[cfg(test)]-gated so the release build carries no dead code. The resolver's public surface is now just resolve_path_plan / resolve_mount_order. The semantic tests it backs (ancestor-wins, sibling isolation, component-prefix matching) are preserved.

SohamDas2021 · 2026-07-14T20:11:27Z

+                ));
+                container.set_config_item("lxc.mount.entry", &mount_entry)?;
+            }
+            FsIntent::Denied => {


Denied file/dir masking gap unaddressed? still rootfs is_file()?

Fixed in bae58f5. The denied branch no longer inspects the rootfs path (<lxc_path>/<name>/rootfs/<container_path>), which doesn't exist until mounts are applied and therefore always classified as a directory. It now classifies from the host path via a new denied_path_is_file helper that uses symlink_metadata (never follows symlinks): a regular host file is masked with /dev/null (create=file), and a directory / symlink / missing host path with an empty read-only tmpfs (create=dir). Added a unit test covering the file / dir / missing host-path cases.

SohamDas2021 · 2026-07-14T20:13:24Z

+/// Exact same-path conflicts (equal [`PathKey`], including entries that differ
+/// only by a trailing separator) collapse to the single most-restrictive
+/// intent; the surviving entry keeps that intent's original path spelling.
+pub fn resolve_path_plan(


Redundant conflict collapse, duplicates normalize_object_conflicts? Upstream duplicate already run by every runner by calling normalize_object_conflicts

Removed in bae58f5. resolve_path_plan no longer re-collapses exact same-path conflicts — that's already done upstream by the config parser's normalize_filesystem_paths (string-level, drops a path from a looser list) and each runner's normalize_object_conflicts (object-identity). The resolver now only orders shallow-to-deep; for any same-path duplicate that still reaches it, denied is added last and sorts last among equal-depth peers, so a backend's last mount at a path wins still yields most-restrictive-wins. Updated the affected unit tests to assert emission-order semantics (via the effective-intent oracle) instead of the removed collapse. cargo test -p wxc_common passes (397).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e5d2aa5b-7f04-4e4d-83d3-a02efe7020ab

Addresses the change requests on PR microsoft#630: - filesystem_mounts.rs: classify denied paths from the HOST path (via symlink_metadata, never following symlinks) instead of the not-yet-existing rootfs path, so a denied host file is masked with /dev/null (create=file) and a directory/symlink/missing path with an empty tmpfs (create=dir). - filesystem_resolve.rs: drop the exact-path most-restrictive collapse from resolve_path_plan; it duplicated the upstream normalize_filesystem_paths (config parser) and normalize_object_conflicts (every runner). The resolver now only orders shallow-to-deep, and emission order still yields most-restrictive-wins for any same-path duplicate that survives upstream. - filesystem_resolve.rs: move the test-only effective_intent oracle into the tests module and gate PathKey::is_prefix_of to test builds. Updated affected unit tests. cargo fmt/clippy clean; wxc_common 398 tests pass; lxc_common + bwrap_common clippy clean on x86_64-unknown-linux-gnu. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

dhoehna · 2026-07-15T23:40:31Z

Addressed the change requests in bae58f5:

effective_intent (filesystem_resolve.rs) — moved into #[cfg(test)] mod tests (test-only oracle) and gated PathKey::is_prefix_of to #[cfg(test)]; no production surface references it.
LXC denied masking (filesystem_mounts.rs) — now classifies from the host path via denied_path_is_file (symlink_metadata, never follows symlinks) instead of the not-yet-existing rootfs path: host file → /dev/null (create=file), directory/symlink/missing → empty tmpfs (create=dir). Added a unit test.
Redundant collapse (resolve_path_plan) — removed; exact same-path conflicts are already resolved upstream by normalize_filesystem_paths (parser) and normalize_object_conflicts (each runner). The resolver now only orders shallow-to-deep, and emission order (denied last) still yields most-restrictive-wins. Tests updated.

Also refreshed the stale PR description (no schema/wire changes remain).

Validation (this is a Windows dev box, so the LXC/bwrap E2E shell suites — which need a Linux host with lxc/bwrap — run in CI, not locally):

cargo fmt --all -- --check — clean
cargo test -p wxc_common — 397 passed
cargo clippy -p wxc_common --all-targets -- -D warnings (Windows) — clean
cargo clippy --target x86_64-unknown-linux-gnu -p lxc_common -p bwrap_common --all-targets -- -D warnings — clean (compiles + lints the shared change against both backends)
node scripts/versioning/validate-configs.js — 186 configs OK against the dev schema

Rebased onto the latest branch tip (post main-merge 037907d) before pushing.

dhoehna · 2026-07-17T17:09:47Z

🧪 Local test re-verification — 2026-07-17

Re-ran the test suites locally at branch tip bae58f5 on a dev workstation. All green (0 failures).

Windows host (x86_64-pc-windows-msvc, cargo 1.96.1):

cargo test -p wxc_common → 397 passed, 0 failed

Linux (WSL2 Ubuntu-24.04, x86_64-unknown-linux-gnu, cargo 1.97.0, isolated CARGO_TARGET_DIR):

cargo test -p lxc_common → 40 passed, 0 failed — the denied-path masking / mount code (filesystem_mounts.rs) ran on Linux (the original Validation only cargo check-compiled it for the Linux target).

Note: the wxc_common count differs slightly from the original Validation because the branch advanced since it was written.

Copilot AI review requested due to automatic review settings July 10, 2026 22:56

dhoehna requested a review from a team as a code owner July 10, 2026 22:56

Copilot started reviewing on behalf of dhoehna July 10, 2026 22:56 View session

Copilot AI reviewed Jul 10, 2026

View reviewed changes

SohamDas2021 reviewed Jul 13, 2026

View reviewed changes

SohamDas2021 requested changes Jul 13, 2026

View reviewed changes

microsoft-github-policy-service Bot assigned dhoehna Jul 13, 2026

microsoft-github-policy-service Bot added the Needs-Author-Feedback Issue needs attention from issue or PR author label Jul 13, 2026

microsoft-github-policy-service Bot added Needs-Attention Issue needs attention from Microsoft and removed Needs-Author-Feedback Issue needs attention from issue or PR author labels Jul 14, 2026

SohamDas2021 requested changes Jul 14, 2026

View reviewed changes

microsoft-github-policy-service Bot added the Needs-Author-Feedback Issue needs attention from issue or PR author label Jul 14, 2026

dhoehna and others added 2 commits July 15, 2026 13:10

Merge microsoft/mxc main (47a172c) into lxc-fs-policy-gaps

037907d

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e5d2aa5b-7f04-4e4d-83d3-a02efe7020ab

SohamDas2021 self-requested a review July 16, 2026 21:38

SohamDas2021 approved these changes Jul 16, 2026

View reviewed changes

Uh oh!

Conversation

dhoehna commented Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Schema

Validation

Microsoft Reviewers: Open in CodeFlow

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SohamDas2021 Jul 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SohamDas2021 commented Jul 13, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SohamDas2021 commented Jul 14, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dhoehna commented Jul 15, 2026

Uh oh!

dhoehna commented Jul 17, 2026

🧪 Local test re-verification — 2026-07-17

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dhoehna commented Jul 10, 2026 •

edited

Loading

SohamDas2021 Jul 13, 2026 •

edited

Loading